Skip to content

reactor: a write must not be truncated, and a finished request must not point at its handle - #263

Merged
EdmondDantes merged 1 commit into
mainfrom
writev-slot-overflow
Aug 21, 2026
Merged

reactor: a write must not be truncated, and a finished request must not point at its handle#263
EdmondDantes merged 1 commit into
mainfrom
writev-slot-overflow

Conversation

@EdmondDantes

@EdmondDantes EdmondDantes commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Defects in the write path, found while reviewing the awaited vectored write (#262).

A write longer than the platform field was truncated and reported as complete

uv_buf_init() takes an unsigned int and narrows anything longer without a word, while
uv_buf_t itself carries a size_t on POSIX and a ULONG on Windows. Every write site
went through it and clamped — libuv_io_write at INT_MAX, libuv_io_writev at
UINT_MAX per slot, libuv_udp_sendto by a cast that wraps — while req->max_size kept
the caller's full length, and the completions report max_size as transferred. An
awaiter comparing the two was told a truncated write had finished.

async_uv_buf_set() fills the fields instead, so nothing is narrowed silently; a length
the platform field cannot hold is refused before submit, on Windows alone.

Separately, uv_write() refuses any batch summing above UV__IO_MAX_BYTES on every
platform, so a larger single write is clamped to that limit and reports the clamped
size
: the caller's loop continues on a short write. A fire-and-forget caller gets a
refusal instead, because free_cb carries no length and a clamp there would drop the tail
in silence.

A finished request kept pointing at a handle that may be gone

The awaiter reads the status after its resume and disposes the request there, and
libuv_io_req_dispose reads req->io. Between the completion and the resume the handle
can be closed and freed — io_close_cb runs in the same loop turn.

The completion now clears req->io when it hands the request to its awaiter, and a
closing handle clears the same pointer in the requests it detaches. Both dispose paths
already guard on req->io != NULL, and the completions reach the handle through
uv_handle_t::data. A reference on the handle was tried first and dropped: it leaks on
the one exit that frees the request without going through dispose, and a shipping caller
reaches that exit by setting free_cb after submit.

Also here

  • A ZSTR batch above 65535 buffers is refused: writev_nbufs is uint16_t, and a wrapped
    count made the completion release nothing — one leaked reference per buffer.
  • A failed uv_udp_send submit no longer leaves send_req.data set, which made
    udp_req_dispose wait forever for a callback that cannot run.
  • Read buffers are clamped rather than narrowed, in both alloc callbacks.
  • libuv_writev_release() replaces three copies of the pre-submit release.

Evidence

  • ext/async/tests: 1104 passed, 0 failed (188 skipped).
  • true-async/server phpt suite against this reactor: 281 passed, 0 failed.
  • The truncation itself has no test: reaching it needs a buffer above 2 GiB.

@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@EdmondDantes EdmondDantes changed the title reactor: a write must not be truncated, and an awaited writev must hold its handle reactor: a write must not be truncated, and a finished request must not point at its handle Aug 21, 2026
@EdmondDantes
EdmondDantes merged commit 523ee10 into main Aug 21, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant